Skip to content

Add itemCount to SentryEnvelopeItemHeader#5230

Merged
denrase merged 15 commits intomainfrom
feat/log-envelope-item
May 19, 2025
Merged

Add itemCount to SentryEnvelopeItemHeader#5230
denrase merged 15 commits intomainfrom
feat/log-envelope-item

Conversation

@denrase
Copy link
Copy Markdown
Collaborator

@denrase denrase commented May 14, 2025

📜 Description

Add itemCount to SentryEnvelopeItemHeader. We need this so log envelopes can be sent from flutter and other hybrid SDKs.

Specs: https://develop.sentry.dev/sdk/telemetry/logs/#log-envelope-item

💡 Motivation and Context

Relates to getsentry/sentry-dart#2919

💚 How did you test it?

Added unit test. Tested in flutter app.

Bildschirmfoto 2025-05-15 um 11 43 07

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

@denrase denrase marked this pull request as draft May 14, 2025 15:45
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 14, 2025

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Add `itemCount` to `SentryEnvelopeItemHeader` ([#5230](https://github.com/getsentry/sentry-cocoa/pull/5230))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 9c9b46e

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 14, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1242.41 ms 1260.08 ms 17.67 ms
Size 23.76 KiB 819.71 KiB 795.95 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
04c0c68 1223.22 ms 1251.24 ms 28.02 ms
ddb4778 1245.36 ms 1254.16 ms 8.80 ms
bdfdf18 1230.33 ms 1240.04 ms 9.72 ms
de46f06 1255.53 ms 1268.44 ms 12.91 ms
d8bb3eb 1230.53 ms 1248.62 ms 18.09 ms
07b120c 1227.48 ms 1235.14 ms 7.66 ms
1b69ee7 1216.53 ms 1226.69 ms 10.16 ms
0ff6aa3 1216.92 ms 1231.44 ms 14.52 ms
2f9c5f9 1218.78 ms 1239.58 ms 20.80 ms
92bc771 1236.60 ms 1245.67 ms 9.07 ms

App size

Revision Plain With Sentry Diff
04c0c68 22.85 KiB 414.09 KiB 391.24 KiB
ddb4778 21.58 KiB 414.92 KiB 393.34 KiB
bdfdf18 22.30 KiB 848.26 KiB 825.95 KiB
de46f06 22.85 KiB 414.74 KiB 391.89 KiB
d8bb3eb 22.31 KiB 765.79 KiB 743.48 KiB
07b120c 21.58 KiB 614.90 KiB 593.32 KiB
1b69ee7 21.58 KiB 707.43 KiB 685.85 KiB
0ff6aa3 23.76 KiB 868.90 KiB 845.14 KiB
2f9c5f9 21.58 KiB 418.82 KiB 397.24 KiB
92bc771 21.58 KiB 699.30 KiB 677.72 KiB

Previous results on branch: feat/log-envelope-item

Startup times

Revision Plain With Sentry Diff
ef7a04d 1225.00 ms 1247.71 ms 22.71 ms
c36b6f2 1209.65 ms 1229.69 ms 20.04 ms

App size

Revision Plain With Sentry Diff
ef7a04d 23.76 KiB 869.47 KiB 845.71 KiB
c36b6f2 23.76 KiB 869.30 KiB 845.54 KiB

@denrase denrase changed the title [TEST] Update envelope item with item count and try to send envelope Add itemCount to SentryEnvelopeItemHeader May 15, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented May 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.816%. Comparing base (765dd70) to head (9c9b46e).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5230       +/-   ##
=============================================
- Coverage   92.895%   92.816%   -0.080%     
=============================================
  Files          683       684        +1     
  Lines        85881     85930       +49     
  Branches     31146     30073     -1073     
=============================================
- Hits         79780     79757       -23     
- Misses        5998      6078       +80     
+ Partials       103        95        -8     
Files with missing lines Coverage Δ
Sources/Sentry/SentryEnvelopeItemHeader.m 97.777% <100.000%> (+0.555%) ⬆️
Sources/Sentry/SentrySerialization.m 98.744% <100.000%> (+0.032%) ⬆️
.../SentryTests/Helper/SentrySerializationTests.swift 99.820% <100.000%> (+0.010%) ⬆️
...Tests/Protocol/SentryEnvelopeItemHeaderTests.swift 100.000% <100.000%> (ø)
...sts/SentryTests/Protocol/SentryEnvelopeTests.swift 95.608% <ø> (-0.417%) ⬇️

... and 25 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 765dd70...9c9b46e. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@denrase
Copy link
Copy Markdown
Collaborator Author

denrase commented May 15, 2025

@buenaflor FYI

@denrase denrase marked this pull request as ready for review May 15, 2025 09:48
Copy link
Copy Markdown
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me but I'll let the cocoa team give the approval

@buenaflor
Copy link
Copy Markdown
Contributor

@denrase UI tests are failing but not sure if it's due to flakiness

@denrase denrase requested review from buenaflor and philprime May 16, 2025 12:23
Copy link
Copy Markdown
Member

@philprime philprime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Thank you for adding the additional test, I did not know that SentryEnvelopeTests already existed, otherwise I would not have proposed adding the SentryEnvelopeItemHeaderTests.

@philprime
Copy link
Copy Markdown
Member

Looks like the formatter bot did not trigger checks (as expected). You can add a comment somewhere, then push another commit and it should trigger CI

@armcknight
Copy link
Copy Markdown
Member

The formatter bot commits are not expected to rerun the full check suite again.

@denrase denrase merged commit d7d4c05 into main May 19, 2025
52 of 54 checks passed
@denrase denrase deleted the feat/log-envelope-item branch May 19, 2025 08:13
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 19, 2025

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentrySerialization.m

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants